RDBC-1018/1019/1023 Fix change tracking metadata aliasing, type-coercion, add per-entity change API#268
Merged
poissoncorp merged 3 commits intoravendb:v7.2from Apr 10, 2026
Merged
Conversation
2bf82da to
09956ab
Compare
09956ab to
87dc26b
Compare
poissoncorp
reviewed
Apr 10, 2026
ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py
Outdated
Show resolved
Hide resolved
87dc26b to
8cd53a6
Compare
poissoncorp
reviewed
Apr 10, 2026
ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py
Outdated
Show resolved
Hide resolved
poissoncorp
reviewed
Apr 10, 2026
ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py
Show resolved
Hide resolved
poissoncorp
reviewed
Apr 10, 2026
| raise ValueError(f"Document {key} must have a Change Vector") | ||
|
|
||
| return cls(key=key, document=document, metadata=metadata, entity=None, change_vector=change_vector) | ||
| return cls(key=key, document=document, metadata=dict(metadata), entity=None, change_vector=change_vector) |
Contributor
There was a problem hiding this comment.
please provide a comment on why we're calling dict here
poissoncorp
reviewed
Apr 10, 2026
poissoncorp
requested changes
Apr 10, 2026
Contributor
poissoncorp
left a comment
There was a problem hiding this comment.
tests look good, check comments though :]
a9acea9 to
33a83ce
Compare
…_info document_info.metadata was the same Python object as document['@metadata'], so _update_metadata_modifications() was mutating the comparison baseline and making metadata-only changes invisible to what_changed() / has_changed(). Store dict(metadata) to break the alias. Regression test: test_change_tracking_metadata.py verifies that modifying or deleting metadata on a stored entity is detected by what_changed(), has_changed(), and has_changes(), and that changes are persisted by save_changes().
Member
Author
I advanced those workflows since we did this ones. The idea was to be able to track C# vs < language > to make it easier. The current flow will keep the tests names in sync so they are easier to verify than to explain what C# does in that regard. Adjusting it. |
…ields) compare_json_array and compare_json both used equality-only comparisons for primitive values, making 1 and '1' (or True and 1) compare equal — silent data loss. Fix: use compare_values(old, new) = old==new AND type(old)==type(new) for all primitive comparisons, both in array elements and top-level scalar fields. The array path had str(old) == str(new). The scalar path had `new_prop == old_prop or compare_values(...)` where compare_values was unreachable dead code (it can only be True when the first operand is already True). Both are replaced with `if JsonOperation.compare_values(old_prop, new_prop): continue`. Regression tests verify that array and scalar fields changing type (int→str, bool→int) are detected as changes and persisted by save_changes().
…ked_entities() C# Advanced.WhatChangedFor(entity) returns DocumentsChanges[] for a single entity. Python had no equivalent. Added _what_changed_for() to InMemoryDocumentSessionOperations and exposed it via Advanced.what_changed_for(). Matches C# behaviour: returns [DocumentDeleted] immediately when the entity has been deleted in the same session, before running the JSON diff. C# Advanced.GetTrackedEntities() returns a dict of all entities currently tracked by the session (stored and deleted). Added _get_tracked_entities() to InMemoryDocumentSessionOperations and exposed it via Advanced.get_tracked_entities(). Each entry maps document ID to a dict with keys: id, entity, is_deleted. Matches C# behaviour: entries deleted by string ID (present in _known_missing_ids but absent from _deleted_entities) appear with is_deleted=True. Regression tests: test_session_advanced_api.py verifies that what_changed_for() returns per-entity changes and correctly reports DocumentDeleted when the entity is deleted before the call; get_tracked_entities() includes newly stored entities with is_deleted=False and entries deleted by string id with is_deleted=True.
33a83ce to
cb4ac17
Compare
poissoncorp
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue link
https://issues.hibernatingrhinos.com/issue/RDBC-1018
https://issues.hibernatingrhinos.com/issue/RDBC-1019
https://issues.hibernatingrhinos.com/issue/RDBC-1023
Additional description
RDBC-1018 –
DocumentInfo.get_new_document_infowas storing a direct reference to themetadatadict from the incoming document, causing mutations to that dict to be reflected back in the session's internal state. Fixed by copying it viadict(metadata).RDBC-1019 – Array-element change tracking compared values using
str(old) == str(new), which incorrectly treated1and"1"as equal. Fixed by requiring both type and value equality.RDBC-1023 – Added two new methods to
session.advanced:what_changed_for(entity)– returns the pending changes for a single tracked entity.get_tracked_entities()– returns a snapshot of all entities currently tracked by the session.Type of change
How risky is the change?
Backward compatibility
Is it platform specific issue?
Documentation update
Documentation Requiredtag.Testing by Contributor
private)Testing by RavenDB QA team
QA Requiredtag.Is there any existing behavior change of other features due to this change?
UI work
Studio Requiredtag.